Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix vi-mode search feature to work similar to Vim #996

Merged
merged 6 commits into from
Aug 24, 2023

Conversation

fukamachi
Copy link
Collaborator

  • Fix the cursor position when searching forward (/), moving to the next match (n)
  • Fix the searching direction of n and N when searching backward (?)
    • n should search backward if the last search was backward

fixes #314

Comment on lines 398 to 399
(unless (funcall *isearch-search-function* (current-point) *isearch-string*)
(move-point (current-point) *isearch-start-point*))
Copy link
Collaborator Author

@fukamachi fukamachi Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed here because I think the current behavior is not intended that the cursor point doesn't go back when the search fails.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see.
if you write the following, the current-point might not be broken when interrupted.

(with-point ((point (current-point)))
  (when (funcall *isearch-search-function* point *isearch-string*)
    (move-point (current-point) point)))

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that looks better. I'll rewrite it as you suggested. Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this has to be the code I originally changed.

For example, when I have a buffer like the following:

[a]pple orange grape

When starting ISearch with C-s lisp, then the cursor moves to app[l]e even though the search is failed.

It's better to go back to the starting point when the search has failed. How do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #1002

Comment on lines +350 to +364
(prog1
(cond ((variable-value 'isearch-next-last :buffer)
(setf (variable-value 'isearch-next-last :buffer) nil)
(with-point ((p (current-point)))
(buffer-start p)
(if (funcall *isearch-search-forward-function* p *isearch-string*)
(progn
(move-point (current-point) p)
t)
nil)))
((not (funcall *isearch-search-forward-function* (current-point) *isearch-string*))
(setf (variable-value 'isearch-next-last :buffer) t)
nil)
(t))
(isearch-update-display))))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to return a boolean value to tell whether the search is successful.

((not (funcall *isearch-search-backward-function* (current-point) *isearch-string*))
(setf (variable-value 'isearch-prev-last :buffer) t)
nil)
(t))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns T. Is this intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I changed this to return a boolean value as I did to isearch-next.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I understand now 👍

@fukamachi fukamachi requested a review from cxxxr August 24, 2023 15:37
@cxxxr
Copy link
Member

cxxxr commented Aug 24, 2023

It looks very good, Thanks!

@cxxxr cxxxr merged commit 47cf8d3 into lem-project:main Aug 24, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vi-mode] The cursor position after n is wrong
2 participants